Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Dec 4, 2024

Motivation:

In (#16) an import for SwiftProtobuf was added to account for well-known types. However, if no well-known types are used then warnings are emitted when the access level is included on the import.

Modifications:

  • Only generate the SwiftProtobuf import for well-known types

Result:

Fewer warnings

Motivation:

In (grpc#16) an import for SwiftProtobuf was added to account for well-known
types. However, if no well-known types are used then warnings are
emitted when the access level is included on the import.

Modifications:

- Only generate the SwiftProtobuf import for well-known types

Result:

Fewer warnings
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Dec 4, 2024
@glbrntt glbrntt requested a review from gjcairo December 4, 2024 18:25
@glbrntt glbrntt enabled auto-merge (squash) December 4, 2024 18:26
@glbrntt glbrntt merged commit 97dfdde into grpc:main Dec 5, 2024
29 checks passed
glbrntt added a commit to glbrntt/grpc-swift-protobuf that referenced this pull request Dec 11, 2024
Motivation:

The fix in grpc#17 relies on `SwiftProtobufPluginLibrary`'s `WellKnownType`
type including all protos bundled by `SwiftProtobuf`, this turns out not
to be true so in some case (like using the "Empty" proto) an import was
missing.

Modifications:

- Use the "isBundleProto" API which better suits our needs
- Update test to use type which would fail test before fix

Result:

Better code gen
glbrntt added a commit that referenced this pull request Dec 12, 2024
Motivation:

The fix in #17 relies on `SwiftProtobufPluginLibrary`'s `WellKnownType`
type including all protos bundled by `SwiftProtobuf`, this turns out not
to be true so in some case (like using the "Empty" proto) an import was
missing.

Modifications:

- Use the "isBundleProto" API which better suits our needs
- Update test to use type which would fail test before fix

Result:

Better code gen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants